-
Notifications
You must be signed in to change notification settings - Fork 16
Make WordPressOrgXMLRPCApiError
conform to CustomNSError
#790
base: trunk
Are you sure you want to change the base?
Conversation
domain: WordPressOrgXMLRPCApiErrorDomain, | ||
code: code, | ||
userInfo: userInfo | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you create a WordPressOrgXMLRPCApiError
error here, cast it to NSError
, and return it to Objective-C code, the instance can be cast back to WordPressOrgXMLRPCApiError
in Swift code when the error is passed from Objective-C code to Swift.
I don't suppose you can cast this NSError
instance to WordPressOrgXMLRPCApiError
? It would be pretty magical if you can. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is a shortsighted leftover from before I realized that the error domain constant could be defined in APIInterface
and therefore be available to packages in both Objective-C and Swift.
I shall remove this utils like I did for WordPressComRestApiErrorDomain
.
This ensures that the error domain is the same between SPM and Framework/CocoaPods builds.
Jumping through this hoop was necessary to avoid the Swift compiler automatically generating an error domain for the `Error` and making it impossible to successfully redefine the domain at the `CustomNSError` conformance site.
22dcdd8
to
ac15e9a
Compare
public struct WordPressOrgXMLRPCApiError: Error { | ||
|
||
/// Error constants for the WordPress XML-RPC API | ||
@objc public enum Code: Int, CaseIterable { | ||
/// An error HTTP status code was returned. | ||
case httpErrorStatusCode | ||
/// The serialization of the request failed. | ||
case requestSerializationFailed | ||
/// The serialization of the response failed. | ||
case responseSerializationFailed | ||
/// An unknown error occurred. | ||
case unknown | ||
} | ||
|
||
let code: Code | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazytonyli I ended up having to wrap the error code enum into an Error
type like you did for the REST API error.
Otherwise, between the @objc
visibility (needed because WordPress-Authenticator uses it 🙄 ) and the Error
conformance, I couldn't find a way to stop the compiler from automatically generating WordPressOrgXMLRPCApiErrorDomain
for @objc public enum WordPressOrgXMLRPCApiError: Int, Error
.
I think this is an okay solution. Apart for the fact that's the only one I could get to work, it's, as far as I can see, the same setup as the other error, which makes the implementation consistent.
failure: { (error, _) in | ||
expect.fulfill() | ||
|
||
XCTAssertTrue(error is WordPressOrgXMLRPCApiError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only change in behavior from the rewrite.
The NSError
-converted error (via asNSerror()
) that the API passes to this failure branch doesn't convert back to WordPressOrgXMLRPCApiError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might have missed something. Does that mean the error
in the failure block was WordPressOrgXMLRPCApiError
, but in this PR it's changed to something else?
See discussion at #782 (comment)
CHANGELOG.md
if necessary.